Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Deprecate rendering templates with . in the name #39164

Merged
merged 1 commit into from
May 6, 2020

Conversation

jhawthorn
Copy link
Member

The last attempt at this change generated unwanted warnings from the dependency tracker, but that should be fixed by #39068.

Allowing templates with . introduces some ambiguity. Is index.html.erb a template named index with format html, or is it a template named index.html without a format? We (humans) know it's probably the former, but if we asked ActionView to render index.html we would currently get some combination of the two: a Template with index.html as the name and virtual path, but with html as the format.

This deprecates having "." anywhere in the template's name, we should reserve this character for specifying formats. I think in 99% of cases this will be people specifying index.html instead of simply index.

This was actually once deprecated in the 3.x series (removed in 6c57177) but I don't think we can rely on nobody having introduced this in the past 8 years 😅.

Allowing templates with "." introduces some ambiguity. Is index.html.erb
a template named "index" with format "html", or is it a template named
"index.html" without a format? We know it's probably the former, but if
we asked ActionView to render "index.html" we would currently get some
combination of the two: a Template with index.html as the name and
virtual path, but with html as the format.

This deprecates having "." anywhere in the template's name, we should
reserve this character for specifying formats. I think in 99% of cases
this will be people specifying `index.html` instead of simply `index`.

This was actually once deprecated in the 3.x series (removed in
6c57177) but I don't think we can rely
on nobody having introduced this in the past 8 years.
@jhawthorn jhawthorn merged commit 342f2f0 into rails:master May 6, 2020
@supairish
Copy link

supairish commented Mar 11, 2021

@jhawthorn Hello! So with this change, what's the workaround for people using multiple template format types in views?

For example, those of us rendering pdf templates i.e. a show.pdf and within there, we want to use a partial from our .html version (_header.html) How do we now render that html template without duplicating the partial as _header.pdf to avoid explicitly specifying the file format?

Within our show.pdf template

old way

render 'header.html'

Now gives the deprecation: DEPRECATION WARNING: Rendering actions with '.' in the name is deprecated

Trying

render partial: 'header', formats: [:html] 

doesn't work and raises

Missing partial _header with {:locale=>[:en], :formats=>[:pdf], :variants=>[], :handlers=>[:raw, :erb, :html, :builder, :ruby, :coffee, :haml, :jbuilder]}

Am I missing something with this change?

@jhawthorn
Copy link
Member Author

Trying

render partial: 'header', formats: [:html] 

doesn't work and raises

Missing partial _header with {:locale=>[:en], :formats=>[:pdf], :variants=>[], :handlers=>[:raw, :erb, :html, :builder, :ruby, :coffee, :haml, :jbuilder]}

Am I missing something with this change?

That should be working. I'm surprised to see :formats=>[:pdf] in your error output

@ghiculescu
Copy link
Member

ghiculescu commented Jul 29, 2021

@supairish we had a similar situation, and in this case we just did render partial: 'header' (no format -> defaults to html).

bobmazanec pushed a commit to rubyforgood/human-essentials that referenced this pull request Dec 17, 2021
to prevent/avoid
  DEPRECATION WARNING: Rendering actions with '.' in the name is
  deprecated: [name] (called from [...])

For more info, see rails/rails#39164
seanmarcia pushed a commit to rubyforgood/human-essentials that referenced this pull request Dec 17, 2021
* Fix Selenium deprecation warning

Prevent
  Selenium [DEPRECATION] [:browser_options] :options as a parameter for
  driver initialization is deprecated.
  Use :capabilities with an Array of value capabilities/options if
  necessary instead.

warning by using `capabilities:` instead of `options:`

Renaming the local variable wasn't strictly necessary, of course, but I
feel it maintains the spirit/pattern of the original code.

* Remove extraneous match option

Addresses/prevents
  The :exact option only has an effect on queries using the XPath#is
   method. Using it with the query "option" has no effect.
advisory text in spec run output

* Update ActiveModel::Errors enumeration

Use a single block
parameter to prevent/resolve
  DEPRECATION WARNING: Enumerating ActiveModel::Errors as a hash has
  been deprecated.
  In Rails 6.1, `errors` is an array of Error objects,
  therefore it should be accessed by a block with a single block
  parameter like this:

    person.errors.each do |error|
      attribute = error.attribute
      message = error.message
    end

  You are passing a block expecting two parameters,
  so the old hash behavior is simulated. As this is deprecated,
this will result in an ArgumentError in Rails 6.2.
  (called from [method] at [path])

* Strip template filename extensions

to prevent/avoid
  DEPRECATION WARNING: Rendering actions with '.' in the name is
  deprecated: [name] (called from [...])

For more info, see rails/rails#39164

* Avoid "expect {}.not_to raise_error" false positive

Avoid/prevent
  WARNING: Using `expect { }.not_to raise_error(SpecificErrorClass)`
  risks false positives, since literally any other error would cause the
  expectation to pass, including those raised by Ruby
  (e.g. `NoMethodError`, `NameError` and `ArgumentError`), meaning the
  code you are intending to test may not even get reached.
  Instead consider using
    `expect { }.not_to raise_error`
  or
    `expect { }.to raise_error(DifferentSpecificErrorClass)`.
  This message can be suppressed by setting:
  `RSpec::Expectations.configuration.on_potential_false_positives = :nothing`.
  Called from [path]/spec/jobs/notify_partner_job_spec.rb:12:
  in `block (3 levels) in <top (required)>

Changing the expecation here seemed simpler/better than the other
suggestion (changing the global RSpec config)

* Apply wait: & avoid warning

Move `wait:` specification into matcher so it both applies and
avoids/prevents
  WARNING: ignoring the provided expectation message argument
  ({:wait=>10}) since it is not a string or a proc.

* Set Partners::User#name in factory

to avoid/prevent
  Checking for expected text of nil is confusing and/or pointless since
  it will always match. Please specify a string or regexp instead.
  [path]/spec/system/partner_system_spec.rb:351

Strictly speaking, only the first one (l. 46) is
required for this; I added the 2nd (l. 57) simply for completeness.

All that said, `partner_user.name` actually gets `Partner` curently
because of the `name: "Partner"` on
spec/system/partner_system_spec.rb:342

Looks like that has been there since this context & example was
introduced in 42e56a0

* Remove spec debug output

Co-authored-by: Bob Mazanec <bob@wizeservices.com>
x1wins added a commit to x1wins/rails-new-with-docker-compose that referenced this pull request Mar 6, 2022
…s deprecated: shippings/autocomplete.json.jbuilder (called from autocomplete at /myapp/app/controllers/shippings_controller.rb:25)

https://stackoverflow.com/questions/68836670/rails-6-1-4-deprecation-warning-rendering-actions-with
rails/rails#39164 (comment)

(cherry picked from commit 43ea26950a754901e51b25bf45dc2ca6f898d148)
hannako added a commit to alphagov/finder-frontend that referenced this pull request Apr 5, 2022
Having an extension in a partial name was deprecated in Rails 6.1 and
removed in Rails 7 (rails/rails#39164)

The file extension parameter passed to the render_as_string needs to
be a symbol. I suspect this wasn't working before, but it didn't matter because
the file name contained the format type.
hannako added a commit to alphagov/finder-frontend that referenced this pull request Apr 5, 2022
Having an extension in a partial name was deprecated in Rails 6.1 and
removed in Rails 7 (rails/rails#39164)

The file extension parameter passed to the render_as_string needs to
be a symbol. I suspect this wasn't working before, but it didn't matter because
the file name contained the format type.
hannako added a commit to alphagov/finder-frontend that referenced this pull request Apr 28, 2022
Having an extension in a partial name was deprecated in Rails 6.1 and
removed in Rails 7 (rails/rails#39164)

The file extension parameter passed to the render_as_string needs to
be a symbol. I suspect this wasn't working before, but it didn't matter because
the file name contained the format type.
hannako added a commit to alphagov/finder-frontend that referenced this pull request May 5, 2022
Having an extension in a partial name was deprecated in Rails 6.1 and
removed in Rails 7 (rails/rails#39164)

The file extension parameter passed to the render_as_string needs to
be a symbol. I suspect this wasn't working before, but it didn't matter because
the file name contained the format type.
chadlwilson added a commit to chadlwilson/gocd that referenced this pull request Aug 17, 2022
chadlwilson added a commit to chadlwilson/gocd that referenced this pull request Aug 17, 2022
dmarcoux pushed a commit to openSUSE/open-build-service that referenced this pull request Oct 27, 2022
Using file extensions in render calls has been deprecated and it is now
unsupported in Rails 7.x.

For details, see rails/rails#39164
njseeto added a commit to ministryofjustice/fb-submitter that referenced this pull request Mar 16, 2023
This was deprecated a few years ago, and was deprecated to
prevent ambiguity in parsing the partial name.

Docs: rails/rails#39164
njseeto added a commit to ministryofjustice/fb-submitter that referenced this pull request Mar 16, 2023
This was deprecated a few years ago, and was deprecated to
prevent ambiguity in parsing the partial name.

Docs: rails/rails#39164
njseeto added a commit to ministryofjustice/fb-submitter that referenced this pull request Mar 16, 2023
This was deprecated a few years ago, and was deprecated to
prevent ambiguity in parsing the partial name.

Docs: rails/rails#39164
D-system added a commit to D-system/rails that referenced this pull request Jan 26, 2024
Since rails#39164 extension are deprecated (shipped with Rails 6.1)
And has been removed in Rails 7.0.
D-system added a commit to D-system/rails that referenced this pull request Jan 30, 2024
Since rails#39164 extension are deprecated (shipped with Rails 6.1).
And has been removed in Rails 7.0.
viralpraxis pushed a commit to viralpraxis/rails that referenced this pull request Mar 24, 2024
Since rails#39164 extension are deprecated (shipped with Rails 6.1).
And has been removed in Rails 7.0.
fractaledmind pushed a commit to fractaledmind/rails that referenced this pull request May 13, 2024
Since rails#39164 extension are deprecated (shipped with Rails 6.1).
And has been removed in Rails 7.0.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants